buffer: optimize buffer.concat performance #61721

mertcanaltin · 2026-02-07T15:13:33Z

Removed the _copyActual indirection in the copy loop and called TypedArrayPrototypeSet directly.
Split auto-length and explicit-length paths so the auto-length copy loop is branch free. Replaced Buffer.allocUnsafe with allocate to skip redundant validation.

benchmark results:

➜  node git:(mert/buffer-concat-optimize-js) ✗ node-benchmark-compare ./result.csv
                                                                            confidence improvement accuracy (*)    (**)   (***)
buffers/buffer-concat-fill.js n=800000 extraSize=1                                   *     10.37 %       ±9.47% ±13.37% ±19.13%
buffers/buffer-concat-fill.js n=800000 extraSize=1024                                       2.91 %       ±6.22%  ±8.53% ±11.62%
buffers/buffer-concat-fill.js n=800000 extraSize=256                                 *      7.05 %       ±6.13%  ±8.41% ±11.49%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=16           **      1.76 %       ±1.18%  ±1.64%  ±2.26%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=4           ***      5.20 %       ±1.21%  ±1.65%  ±2.25%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=16          **      1.96 %       ±1.17%  ±1.61%  ±2.21%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=4          ***      3.06 %       ±1.50%  ±2.06%  ±2.84%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=16                 1.93 %       ±8.35% ±11.48% ±15.73%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=4                  2.37 %       ±6.69%  ±9.34% ±13.12%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=16                  -0.59 %       ±0.99%  ±1.36%  ±1.86%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=4           ***      5.17 %       ±1.25%  ±1.71%  ±2.33%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=16                  0.98 %       ±1.45%  ±2.02%  ±2.81%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=4          ***      4.44 %       ±1.89%  ±2.62%  ±3.62%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=16          *     -1.97 %       ±1.89%  ±2.63%  ±3.67%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=4         ***      3.55 %       ±1.08%  ±1.49%  ±2.04%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 15 comparisons, you can thus expect the following amount of false-positive results:
  0.75 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.15 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)
➜  node git:(mert/buffer-concat-optimize-js) ✗

codecov · 2026-02-07T16:19:16Z

Codecov Report

❌ Patch coverage is 90.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.74%. Comparing base (3ab9dd8) to head (4848305).
⚠️ Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/buffer.js	90.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61721      +/-   ##
==========================================
+ Coverage   89.73%   89.74%   +0.01%     
==========================================
  Files         675      675              
  Lines      204502   204673     +171     
  Branches    39304    39329      +25     
==========================================
+ Hits       183502   183684     +182     
+ Misses      13283    13281       -2     
+ Partials     7717     7708       -9

Files with missing lines	Coverage Δ
lib/buffer.js	`99.16% <90.00%> (-0.63%)`	⬇️

... and 52 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ChALkeR

See #60399 (comment)

Shouldn't _copyActual be just using TypedArrayPrototypeSet since #60399?
cc @gurgunday

lib/buffer.js

gurgunday

Thanks for the ping @ChALkeR

Yeah so other than inlining what we did in _copyActual (#60399), this PR avoids going through _copy even in the non sourceStart === 0 && nb === sourceLen (partial copy) case (I remember that we'd talked about doing this in a follow-up PR anyway)

I like that change

lib/buffer.js

gurgunday · 2026-02-07T18:56:43Z

lib/buffer.js

+    if (pos < length) {
+      TypedArrayPrototypeFill(buffer, 0, pos, length);
+    }


Can we even hit this part? I think this is unreachable because you moved type validation to the beginning and it will always fill after going through all of them?

It's still reachable if a buffer gets detached between the validation and copy loops buf.length becomes 0, pos won't advance, leaving uninitialized bytes without the zero-fill.

Not really sure how it can be detached unless a length getter is compromised. It's all synchronous code here?

In any case, it would be nice to have coverage for it

Other than coverage, the PR LGTM

How about an assert here?

(Attempting to read from a detached buffer should throw an error from the engine in any case.)

If the buffer gets detached between these points should actually cause the Uint8Array.prototype.set operation to fail.

Consider the example:

const u8_1 = new Uint8Array([1,2,3,4]); const u8_2 = new Uint8Array([5,6,7,8]); let called = false; Object.defineProperty(u8_1, 'length', { get() { // The first time this is called, return the actual length of the array // The second time this is called, we'll also transfer the ArrayBuffer of the second if (!called) { called = true; } else { u8_2.buffer.transfer(); } return 4; }, }); const buf = Buffer.concat([u8_1, u8_2]); console.log(buf);

node:buffer:631 TypedArrayPrototypeSet(buffer, buf, pos); ^ TypeError: Cannot perform %TypedArray%.prototype.set on a detached ArrayBuffer at Buffer.set (<anonymous>) at Buffer.concat (node:buffer:631:7) at Object.<anonymous> (/home/jsnell/tmp/fubar.js:20:20) at Module._compile (node:internal/modules/cjs/loader:1811:14) at Object..js (node:internal/modules/cjs/loader:1942:10) at Module.load (node:internal/modules/cjs/loader:1532:32) at Module._load (node:internal/modules/cjs/loader:1334:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 Node.js v26.0.0-pre

I'm all for being defensive here tho.

TypedArrayPrototypeGetLength would be the tonic there, presumably.

Probably. Also just make sure that Uint8Array.prototype.set itself does not call the user overide getter (it shouldn't.. but let's confirm)

Thanks, switched to TypedArrayPrototypeGetByteLength to avoid the spoofed length getter. Zero-fill kept as defensive measure.

I would be sure to also add a test that covers the spoofed length test.

I added now, thanks

lib/buffer.js

mertcanaltin · 2026-02-07T19:52:28Z

I updated bench result: #61721 (comment)

lib/buffer.js

Renegade334 · 2026-02-10T00:23:22Z

lib/buffer.js

+    if (pos < length) {
+      TypedArrayPrototypeFill(buffer, 0, pos, length);
+    }


Nit: my preference would still be for an assert here, since pos !== length is de facto an invalid state.

I'd prefer keeping the zero-fill here if we're ever wrong about it being unreachable, zero-filling is safer than crashing the process with uninitialized memory in the buffer.

lib/buffer.js

gurgunday

lgtm!

buffer: optimize buffer.concat performance

7ded458

nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run. labels Feb 7, 2026

ChALkeR reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Show resolved Hide resolved

mertcanaltin added 2 commits February 7, 2026 19:41

revert comment lines

a7fd75d

revert comment lines

885c7a3

benjamingr reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Outdated Show resolved Hide resolved

revert listLen caching

ba806ae

gurgunday added the needs-benchmark-ci PR that need a benchmark CI run. label Feb 7, 2026

gurgunday reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Outdated Show resolved Hide resolved

move type validation before allocation

559f5e9

Renegade334 reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Outdated Show resolved Hide resolved

use TypedArrayPrototypeGetByteLength

1898304

Renegade334 approved these changes Feb 10, 2026

View reviewed changes

add test for spoofed length getter in concat

4848305

Renegade334 approved these changes Feb 10, 2026

View reviewed changes

gurgunday approved these changes Feb 10, 2026

View reviewed changes

Uh oh!

buffer: optimize buffer.concat performance #61721

Are you sure you want to change the base?

buffer: optimize buffer.concat performance #61721

Conversation

mertcanaltin commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ChALkeR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gurgunday left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gurgunday Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mertcanaltin commented Feb 7, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mertcanaltin Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gurgunday left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mertcanaltin commented Feb 7, 2026 •

edited

Loading

codecov bot commented Feb 7, 2026 •

edited

Loading

gurgunday left a comment •

edited

Loading

gurgunday Feb 7, 2026 •

edited

Loading

jasnell Feb 8, 2026 •

edited

Loading

mertcanaltin Feb 10, 2026 •

edited

Loading